crates/katana/trie/src/contracts.rs (1)

13-16: Ohayo sensei! Bonsai identifier usage comment is helpful.
It clarifies how multiple tries might share an underlying database.

crates/katana/rpc/rpc-types-builder/src/state_update.rs (1)

33-38: Ohayo sensei! New root derivation from a historical provider.
The code handles optional returns properly via expect. However, be mindful that a failing expect leads to a panic. Consider error handling or logging for robust production code.

crates/katana/trie/src/classes.rs (1)

14-20: Ohayo sensei! verify method looks tidy.
Leverages the underlying Pedersen verification. Potential future extension: handle partial success or filtering out invalid proofs.

crates/katana/storage/db/src/models/trie.rs (3)

24-35: Decompression bounds check, sensei!
You’re slicing the byte array for the value. Watch for partial or corrupted data. Consider extra checks or graceful error handling.

---

86-86: Panic on empty buffer, sensei.
Consider returning an error instead of panicking, especially if inputs can be external.

---

95-97: Buffer length check, sensei!
Nicely done. Again, consider an error over panic for production resilience.

crates/katana/trie/src/lib.rs (4)

8-8: Re-exporting crates, sensei!
Exposing bitvec and bonsai_trie under bonsai simplifies imports for consumers.

---

61-63: Root retrieval method, ohayo sensei!
root_hash can fail if the ID is missing. The expect usage will panic. Consider an error instead.

---

Line range hint 96-108: Merkle root computation, sensei!
anyhow::Result provides flexible error handling. If performance matters, consider a specialized error type.

---

133-140: verify_proof function design is ohayo!
This direct approach is more user-friendly. Potential improvement: bubble up the error from .unwrap().

crates/katana/storage/db/src/abstraction/transaction.rs (2)

94-128: DbTxMutRef trait extends the reference approach.
Enables mutation while avoiding full Tx ownership. Consider clarifying concurrency boundaries.

---

130-149: Implementation for &Tx: DbTxRef, sensei!
Smart reuse of existing trait methods. Return proper errors instead of panics if nested references are not allowed.

crates/katana/rpc/rpc-types/src/trie.rs (3)

27-49: MerkleNode enum is well-designed.
Ohayo sensei! The Edge variant’s length can benefit from a bounded integer check if you expect it never to exceed 251. But everything else is clean.

---

79-124: Proof structs (ClassesProof, ContractsProof, etc.) are well-labeled.
Hey sensei, consider adding doc comments on how each proof set is expected to be used for future maintainers.

---

126-144: Conversions from MultiProof to Nodes and vice versa are straightforward.
Take note that the mapping approach in .into_iter().map(...) might fail silently if unexpected data is passed in. Might add validation of node types if needed.

crates/katana/storage/db/src/trie/snapshot.rs (1)

35-47: recent_change_from_block helper logic.
Covers edge cases well. Consider clarifying the fallback behavior in doc comments if no changes are found.

bin/katana/src/cli/init/mod.rs (3)

121-199: prompt method.
Ohayo sensei! This interactive approach is user-friendly. Consider improved error messaging if the contract existence check fails for other reasons.

---

202-281: init_core_contract function.
Nice spinner-based feedback. Watch out for potential edge cases if the contract is partially declared. Otherwise, looks great.

---

303-319: Configuration path generation.
Direct usage of dirs::config_local_dir() might behave differently across OSes. That’s acceptable, but note it in your docs.

crates/katana/storage/provider/src/providers/db/state.rs (2)

8-9: Added import for TrieDbFactory.
Let's keep it pinned to the same version used by the rest of the codebase.

---

342-379: impl StateProofProvider for HistoricalStateProvider<Tx>.
The usage of historical(self.block_number) is a neat approach. The .expect("should exist") might be risky if the DB is missing data. Could degrade user experience.

crates/katana/storage/db/src/trie/mod.rs (1)

Line range hint 236-364: Ohayo sensei!
Implementing BonsaiPersistentDatabase with snapshot and merge placeholders is strategic. Consider finalizing merge logic soon.

crates/katana/node/src/config/rpc.rs (1)

13-14: Ohayo sensei! There's a small nitpick in the doc comment.
The doc comment says "Default maximmum number of keys," but "maximmum" should be "maximum."

-/// Default maximmum number of keys for the `starknet_getStorageProof` RPC method.
+/// Default maximum number of keys for the `starknet_getStorageProof` RPC method.

crates/katana/rpc/rpc/src/starknet/read.rs (1)

270-282: Ohayo sensei! Implementation of get_storage_proof is logically sound.
Consider adding logging for failure scenarios in get_proofs to ease debugging. Also, ensure unit tests cover large input arrays.

crates/katana/storage/provider/tests/block.rs (3)

14-14: Ohayo sensei! Possibly unused import of StateRootProvider.
It seems StateRootProvider might no longer be needed given the broader refactor away from the old trait. Consider removing it if it's truly obsolete.

-use katana_provider::traits::state::{StateFactoryProvider, StateRootProvider};
+use katana_provider::traits::state::StateFactoryProvider;

---

32-32: Ohayo sensei! This test is ignored due to missing trie computation.
Eventually, you might want to revisit and implement trie logic for fork mode to ensure consistent test coverage. Let me know if you need help!

---

49-49: Ohayo sensei! Another ignored test for fork provider.
Similar to the previous comment, recheck trie computation logic to enable this test.

crates/katana/storage/provider/src/providers/fork/mod.rs (1)

413-423: Ohayo sensei! Consider removing commented-out code if it's truly deprecated.
Fully removing the old StateRootProvider code would help keep the file tidy and reduce confusion.

-// impl StateRootProvider for ForkedProvider {
-//     ...
-//}

crates/katana/executor/src/implementation/blockifier/state.rs (1)

269-277: Ohayo sensei! The StateRootProvider trait is similarly unimplemented.
If there’s no short-term plan to implement it, keep it well-documented to avoid surprises.

crates/katana/storage/provider/src/providers/fork/backend.rs (1)

665-688: Ohayo sensei! The unimplemented state-proof methods are clearly handled.
If there's a roadmap for these features, consider a detailed TODO.

Would you like me to open a new issue discussing possible multiproof approaches?

bin/katana/Cargo.toml (1)

17-31: Ohayo sensei! Let's review the version pinning strategy.

I notice inconsistent version pinning across dependencies:

• Some are workspace-managed (.workspace = true)
• Others are explicitly versioned (e.g., byte-unit = "5.1.4", inquire = "0.7.5")

Consider moving more dependencies to workspace-level version management for better maintainability. This approach would:

• Ensure version consistency across the workspace
• Simplify dependency updates
• Reduce the risk of version conflicts

Cargo.toml (1)

280-284: Ohayo sensei! Macro dependencies look well-configured.

The addition of standard Rust macro development crates (proc-macro2, quote, syn) with appropriate version constraints is good. These are essential for developing procedural macros.

However, consider adding a comment explaining the intended use of these macro dependencies for better maintainability.

 # macro
+# These dependencies are used for developing procedural macros across the project
 proc-macro2 = "1.0"
 quote = "1.0"
 syn = { version = "2.0", default-features = false }

crates/katana/storage/provider/src/providers/db/trie.rs (1)

43-102: 🛠️ Refactor suggestion

Insertion of contract updates, sensei!
This method carefully commits storage changes, updates leaf structures, and finalizes the contract trie in a single transaction. The synergy is good. Keep an eye on transaction concurrency or partial commits if block_number increments fail or get interrupted.

crates/katana/trie/src/lib.rs (1)

71-84: 🛠️ Refactor suggestion

Insertion and commit logic, ohayo.
The bit-slice approach is consistent. The unwrap calls might cause runtime panics if the DB is offline. Maybe gracefully handle DB errors.

crates/katana/contracts/Makefile (2)

16-17: ⚠️ Potential issue

Ohayo sensei! Watch out for variable shadowing

The ORIGINAL_CLASS_NAME variable is being redefined, which could lead to conflicts with the account contract section above.

Consider using unique variable names:

-ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX)
+PILTOVER_ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

PILTOVER_ORIGINAL_CLASS_NAME := piltover_appchain$(CONTRACT_CLASS_SUFFIX)
CLASS_NAME := appchain_core_contract.json

---

19-21: 🛠️ Refactor suggestion

Build directory target might need coordination

The build target uses the same directory and doesn't clean previous builds, which could lead to file conflicts.

Consider adding clean steps and using separate build targets:

-$(BUILD_DIR): ./piltover/src/*
+build-piltover: $(BUILD_DIR) ./piltover/src/*
+	rm -f $(BUILD_DIR)/$(CLASS_NAME)
 	cd piltover && scarb build
 	mv target/dev/$(ORIGINAL_CLASS_NAME) $(BUILD_DIR)/$(CLASS_NAME)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

build-piltover: $(BUILD_DIR) ./piltover/src/*
	rm -f $(BUILD_DIR)/$(CLASS_NAME)
	cd piltover && scarb build
	mv target/dev/$(ORIGINAL_CLASS_NAME) $(BUILD_DIR)/$(CLASS_NAME)